Skip to content

pylint: fix misplaced-bare-raise #49018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Oct 11, 2022
Merged

pylint: fix misplaced-bare-raise #49018

merged 4 commits into from
Oct 11, 2022

Conversation

vamsi-verma-s
Copy link
Contributor

@slackline slackline mentioned this pull request Oct 10, 2022
Closed
@mroeschke mroeschke added the Code Style Code style, linting, code_checks label Oct 10, 2022
akx
akx reviewed Oct 10, 2022
View reviewed changes
pandas/core/internals/blocks.py
@@ -1983,7 +1983,7 @@ def _catch_deprecated_value_error(err: Exception) -> None:
# IntervalDtype mismatched 'closed'
pass
elif "Timezones don't match" not in str(err):
raise
raise err
Copy link
Contributor

@akx akx Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would raise the exception anew and so lose the original traceback. Is that on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed that, it wasn't the intention. Do you think something like below would suffice?

Suggested change
raise err
raise ValueError(err) from err

MarcoGorelli
MarcoGorelli suggested changes Oct 10, 2022
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please wait to merge, I think this is might be a bug in pylint

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Oct 10, 2022 •
edited
Loading

OK seems like pylint just doesn't detect this, as _catch_deprecated_value_error is always used within an except context

e.g. pylint would also raise on

def raise_exception(err: Exception):
    raise

try:
    1/0
except ZeroDivisionError as err:
    raise_exception(err)

I think we can just ignore this inline, referencing this PR:

raise  # pylint: disable=misplaced-bare-raise GH49018

@MarcoGorelli
Copy link
Member

Hi @akx - are you sure about your raise err losing the traceback? I just tried it, and with raise err the traceback looks the same

With just raise:

Traceback (most recent call last):
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1116, in __setitem__
    self._set_with_engine(key, value)
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1186, in _set_with_engine
    loc = self.index.get_loc(key)
  File "/home/marcogorelli/pandas-dev/pandas/core/indexes/range.py", line 395, in get_loc
    self._check_indexing_error(key)
  File "/home/marcogorelli/pandas-dev/pandas/core/indexes/base.py", line 6028, in _check_indexing_error
    raise InvalidIndexError(key)
pandas.errors.InvalidIndexError: [False  True False]

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "t.py", line 51, in <module>
    ser[mask] = 1
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1173, in __setitem__
    self._where(~key, value, inplace=True)
  File "/home/marcogorelli/pandas-dev/pandas/core/generic.py", line 9781, in _where
    new_data = self._mgr.putmask(mask=cond, new=other, align=align)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/managers.py", line 410, in putmask
    return self.apply(
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/managers.py", line 350, in apply
    applied = getattr(b, f)(**kwargs)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/blocks.py", line 1534, in putmask
    _catch_deprecated_value_error(err)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/blocks.py", line 1532, in putmask
    values._putmask(mask, new)
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/string_.py", line 421, in _putmask
    ExtensionArray._putmask(self, mask, value)
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/base.py", line 1517, in _putmask
    self[mask] = val
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/string_.py", line 404, in __setitem__
    raise ValueError(
ValueError: Cannot set non-string value '1' into a StringArray.

with raise err:

Traceback (most recent call last):
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1116, in __setitem__
    self._set_with_engine(key, value)
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1186, in _set_with_engine
    loc = self.index.get_loc(key)
  File "/home/marcogorelli/pandas-dev/pandas/core/indexes/range.py", line 395, in get_loc
    self._check_indexing_error(key)
  File "/home/marcogorelli/pandas-dev/pandas/core/indexes/base.py", line 6028, in _check_indexing_error
    raise InvalidIndexError(key)
pandas.errors.InvalidIndexError: [False  True False]

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "t.py", line 51, in <module>
    ser[mask] = 1
  File "/home/marcogorelli/pandas-dev/pandas/core/series.py", line 1173, in __setitem__
    self._where(~key, value, inplace=True)
  File "/home/marcogorelli/pandas-dev/pandas/core/generic.py", line 9781, in _where
    new_data = self._mgr.putmask(mask=cond, new=other, align=align)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/managers.py", line 410, in putmask
    return self.apply(
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/managers.py", line 350, in apply
    applied = getattr(b, f)(**kwargs)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/blocks.py", line 1534, in putmask
    _catch_deprecated_value_error(err)
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/blocks.py", line 1986, in _catch_deprecated_value_error
    raise err # pylint: disable=misplaced-bare-raise GH49018
  File "/home/marcogorelli/pandas-dev/pandas/core/internals/blocks.py", line 1532, in putmask
    values._putmask(mask, new)
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/string_.py", line 421, in _putmask
    ExtensionArray._putmask(self, mask, value)
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/base.py", line 1517, in _putmask
    self[mask] = val
  File "/home/marcogorelli/pandas-dev/pandas/core/arrays/string_.py", line 404, in __setitem__
    raise ValueError(
ValueError: Cannot set non-string value '1' into a StringArray.

So, I think it's fine to just do raise err and not have to ignore any checks? (apologies @vamsi-verma-s for conflicting requests)

Code to trigger this:

ser = pd.Series(["a", "b", "c"], dtype='string')
mask = np.array([False, True, False])
ser[mask] = 1

@vamsi-verma-s
Copy link
Contributor Author

Hi @MarcoGorelli

I have reverted to original change. I was a bit confused here over loosing traceback.
If my understanding is correct, since we are raising with the same exception instance, the traceback would still be attached to it and don't need to do something like raise ValueError('same error').with_traceback(err.__traceback__)

MarcoGorelli
MarcoGorelli approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vamsi-verma-s

@MarcoGorelli MarcoGorelli added this to the 1.6 milestone Oct 11, 2022
@MarcoGorelli MarcoGorelli merged commit cdb905a into pandas-dev:main Oct 11, 2022
@vamsi-verma-s
Copy link
Contributor Author

Thank you all for reviewing this.

@vamsi-verma-s vamsi-verma-s deleted the fix-48855 branch October 11, 2022 07:22
@mroeschke mroeschke modified the milestones: 1.6, 2.0 Oct 13, 2022
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
@vamsi-verma-s @noatamir
pylint: fix misplaced-bare-raise (pandas-dev#49018)
531fded 
* pylint: fix misplaced-bare-raise

* ignore pylint misplaced-bare-raise

* Revert "ignore pylint misplaced-bare-raise"

This reverts commit d633101.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akx akx akx left review comments

@mroeschke mroeschke mroeschke approved these changes

@MarcoGorelli MarcoGorelli MarcoGorelli approved these changes

Assignees
No one assigned
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@vamsi-verma-s @MarcoGorelli @akx @mroeschke